Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eventing): Emit new annotations_staged_change event #570

Merged
merged 2 commits into from
Sep 8, 2020

Conversation

ConradJChan
Copy link
Contributor

@ConradJChan ConradJChan commented Sep 1, 2020

TODO:

  • unit tests

src/@types/events.ts Outdated Show resolved Hide resolved
[setActiveAnnotationIdAction.toString()]: handleActiveAnnotationEvents,
[setIsInitialized.toString()]: handleAnnotationsInitialized,
[setStagedAction.toString()]: handleSetStagedAction,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle setStatusAction? Or can it even take the place of setStagedAction here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stick to setStagedAction since the transitions in state for staged allows us to infer the event status as well as the annotation type

src/store/eventing/middleware.ts Outdated Show resolved Hide resolved
Comment on lines +18 to +19
if (prevStaged === null && nextStaged !== null) {
status = 'create';
}

if (prevStaged !== null && nextStaged !== null) {
status = 'update';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we handle if (prevStaged !== null && nextStaged === null)? Maybe states is cancel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it off since our pattern of usage for cancel is to use resetCreatorAction.

@ConradJChan ConradJChan marked this pull request as ready for review September 8, 2020 17:57
@ConradJChan ConradJChan requested a review from a team as a code owner September 8, 2020 17:57
Copy link
Collaborator

@jstoffan jstoffan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests?

@ConradJChan ConradJChan merged commit a8b1938 into box:master Sep 8, 2020
@ConradJChan ConradJChan deleted the staged-change-event branch September 8, 2020 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants